Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli/command: deprecate CopyToFile and reimplement with atomicwriter #5914

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

thaJeztah
Copy link
Member

- Human readable description for the release notes

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah added status/2-code-review kind/refactor PR's that refactor, or clean-up code labels Mar 9, 2025
@codecov-commenter
Copy link

codecov-commenter commented Mar 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.44%. Comparing base (2b84421) to head (239e6ca).

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #5914       +/-   ##
===========================================
- Coverage   59.14%   44.44%   -14.71%     
===========================================
  Files         355        6      -349     
  Lines       29738      261    -29477     
===========================================
- Hits        17590      116    -17474     
+ Misses      11173      139    -11034     
+ Partials      975        6      -969     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This function was exported in e43c792
for use of "docker app", which is now deprecated. The signature of this
function also depended on a non-exported type so it could not be used
externally.

Make it internal again, as it was never designed to be exported. There
are no known external consumers of this function.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
This function was exported in e43c792
for use of "docker app", which is now deprecated. The signature of this
function also depended on a non-exported type, so it could not be used
externally.

Make it internal again, as it was never designed to be exported. There
are no known external consumers of this function.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
This function was exported in 812f113
for use in other parts of the CLI, but it's now only used locally.

Make it internal again, as it was never designed to be exported. There
are no known external consumers of this function, but deprecating it
first, in case there are.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Same functionality, but implemented with atomicwriter. There's a slight
difference in error-messages produced (but can be adjusted if we want).

Before:

    docker image save -o ./no/such/foo busybox:latest
    failed to save image: invalid output path: directory "no/such" does not exist

    docker image save -o /no/permissions busybox:latest
    failed to save image: stat /no/permissions: permission denied

After:

    docker image save -o ./no/such/foo busybox:latest
    failed to save image: invalid file path: stat no/such: no such file or directory

    docker image save -o /no/permissions busybox:latest
    failed to save image: failed to stat output path: lstat /no/permissions: permission denied

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Same functionality, but implemented with atomicwriter. There's a slight
difference in error-messages produced (but can be adjusted if we want).

Before:

    docker container export -o ./no/such/foo mycontainer
    failed to export container: invalid output path: directory "no/such" does not exist

    docker container export -o /no/permissions mycontainer
    failed to export container: stat /no/permissions: permission denied

After:

    docker container export -o ./no/such/foo mycontainer
    failed to export container: invalid file path: stat no/such: no such file or directory

    docker container export -o /no/permissions mycontainer
    failed to export container: failed to stat output path: lstat /no/permissions: permission denied

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/refactor PR's that refactor, or clean-up code status/2-code-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants